[OSDOCS#20691]: Fencing parameters tuning for TNF - #118946
[OSDOCS#20691]: Fencing parameters tuning for TNF#118946bjahagir-OpenShift wants to merge 1 commit into
Conversation
openshift-ci
Bot
added
the
size/XL
1ae1327 to
2451f19
Compare
2451f19 to
e602a90
Compare
|
@bjahagir-OpenShift: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
lucaconsalvi
left a comment
lucaconsalvi
left a comment
There was a problem hiding this comment.
Thanks for putting this together, Bhushan — this is a genuinely useful addition and the structure is really clear. I checked the technical content against the cluster-etcd-operator source: the parameter defaults, the fencing-risk classifications, the CEO overwrite matrix, and the pcmk_delay_base warning all hold up. The revert procedure is a particularly good catch, since deleting the MachineConfig genuinely doesn't undo the pcs changes.
One thing I wanted to check with you: I couldn't find the new assembly in _topic_maps/_topic_map.yml, and the page seems to 404 on the preview (.../installing_tnf/tuning-fencing-parameters-tnf), with the left nav still showing just the four existing TNF pages. Is that entry maybe landing in a separate PR? If not, I think it would need adding here for the page to get built.
Two smaller structural notes:
- Nothing currently xrefs the new page, so even once it is in the nav, someone troubleshooting spurious fencing from "Post-installation troubleshooting and recovery" won't be pointed to it. An xref from there (and/or from "Operating a degraded two-node OpenShift cluster with fencing") would help discoverability.
- All eight files are missing a trailing newline.
Inline comments below — all small stuff apart from the topic map. Happy to re-review once updated.
| @@ -0,0 +1,31 @@ | |||
| :_mod-docs-content-type: ASSEMBLY | |||
There was a problem hiding this comment.
The new assembly doesn't appear in _topic_maps/_topic_map.yml, so I don't think it gets built — the preview 404s at .../installing_tnf/tuning-fencing-parameters-tnf, and the left nav still shows only the four existing TNF pages.
If it isn't coming in a separate PR, it would need an entry under the installing_tnf Topics list (currently lines 444-451):
- Name: Tuning the fencing parameters for a two-node OpenShift cluster with fencing
File: tuning-fencing-parameters-tnfDistros is inherited from the parent installing_tnf entry, so it shouldn't be needed here.
| @@ -0,0 +1,24 @@ | |||
| // Module included in the following assemblies: | |||
| // * installing/installing_two_node_cluster/installing_tnf/tuning-parameters-tnf.adoc | |||
There was a problem hiding this comment.
This points to tuning-parameters-tnf.adoc, but the assembly is actually named tuning-fencing-parameters-tnf.adoc. The existing TNF modules use the full path — for example modules/command-line-options-for-fencing-validator-script.adoc:
// * installing/installing_two_node_cluster/installing_tnf/install-post-tnf.adoc
So this one would be:
// * installing/installing_two_node_cluster/installing_tnf/tuning-fencing-parameters-tnf.adoc
The other five new modules have the same issue with a different spelling (installing_tnf/tuning-tnf-fencing-parameters.adoc) — probably worth fixing all six together.
|
|
||
| :_mod-docs-content-type: CONCEPT | ||
| [id="tuning-tnf-overview_{context}"] | ||
| = How fencing parameter tuning works in a two-node Openshift cluster with fencing |
There was a problem hiding this comment.
Small typo: "Openshift" -> "OpenShift".
|
|
||
| .Procedure | ||
|
|
||
| . Save the `apply-fencing-tuning.sh` script to a local file. |
There was a problem hiding this comment.
The apply-fencing-tuning.sh script body only appears in "Applying a fencing tuning preset". Since these are peer sections, a reader who lands here (or on the revert procedure) has no script to save and no link to where it's defined.
It might be worth moving the script into the overview module as a shared artifact, or adding an xref from here and from "Reverting fencing tuning to defaults" back to where it lives.
Very minor: this module lists "You have the oc CLI tool installed" as a prerequisite, but the revert module omits it.
4 participants
Version(s):
5.0
Issue:
https://redhat.atlassian.net/browse/OSDOCS-20691
Link to docs preview:
https://118946--ocpdocs-pr.netlify.app/openshift-enterprise/latest/installing/installing_two_node_cluster/installing_tnf/install-post-tnf#fencing-validator-script-overview_install-post-tnf
QE review:
Additional information: